Remove activesupport due to its volatility and potential to generate … by tpowell-progress · Pull Request #211 · chef/chef-licensing

tpowell-progress · 2025-10-16T22:02:29Z

…irreconcilable conflicts

Resolves #195 and indirectly resolves #198

Description

activesupport is volatile and chef-licensing makes minimal usage of it. When CVE fixes are issued for some of the web of projects but not others, we get the following:

Fetching gem metadata from https://rubygems.org/.
Resolving dependencies..............................................................................................................................................................
Bundler could not find compatible versions for gem "activesupport":
  In Gemfile:
    chef was resolved to 19.1.90, which depends on
      chef-licensing (>= 1.2.0) was resolved to 1.2.0, which depends on
        activesupport (~> 7.2, >= 7.2.2.1)
 
    oc-chef-pedant was resolved to 2.2.0, which depends on
      activesupport (>= 4.2.7.1, <= 7.1.3.2)
bundler: failed to load command: tasks/bin/run_external_test (tasks/bin/run_external_test)

The Gemfile.lock updated a decent amount, but those should be for development purposes only.

Related Issue

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

I have read the CONTRIBUTING document.
I have run the pre-merge tests locally and they pass.
I have updated the documentation accordingly.
I have added tests to cover my changes.
If Gemfile.lock has changed, I have used --conservative to do it and included the full output in the Description above.
All new and existing tests passed.
All commits have been signed-off for the Developer Certificate of Origin.

jaymzh

🎉

jaymzh · 2025-10-17T17:35:39Z

You have a minor lint failure, but other than that, this looks ready to go.

rahulgoel1 · 2025-10-22T17:05:28Z

@Nik08 @Vasu1105 can you pls review/ approve pls ? This is going to block chef-19 upgrade for inspec 7

Nik08 · 2025-10-24T07:52:18Z

@tpowell-progress @johnmccrae The changes look good. I have 2 concerns here:

Can we separate the lint changes from this PR since they are already covered here Updating linting and cookstyle support #212? This will make the code review much easier and specific to the intended change, which is the activesupport replacement
I see moneta is being used as a compatible storage with Faraday::HttpCache. Will it be possible to cover some test cases which also show the proper functioning of this with Faraday::HttpCache? Or have you done a round of testing with the airgap environment and these changes in caching? We do use headers, which are useful for expiration via Faraday::HttpCache, and just wanted to make sure that functionality is not affected by this integration with Moneta.

sathish-progress

This PR has too many changes that is out of scope for removal of activesupport dependency that is most probably coming from #212. Either this PR should be rebased against main or against jfm/chef19-cookstyle.

sathish-progress · 2025-10-24T09:58:47Z

@@ -0,0 +1,123 @@
+name: Ruby Tests


Please move the tests from here to spec files. We have existing verify pipelines which will tc of running these specs

I see the tests are added in the spec. We don't need this.

Weird. Something must not have been firing because I wasn't getting any results in GA initially.

removed the GitHub workflow in e98a83f

sathish-progress · 2025-10-24T10:10:40Z


 module ChefLicensing
  class ListLicenseKeys
+    using StringRefinements


nitpick, I see we use string refinements only here, and only for the purpose of pluralize may be we should just call it as a util and use the util directly something like below,
Chef::Licensing::StringUtils.present?(str)

The reason I did it as a string refinement was because was one of the prior uses for ActiveSupport. I'm absolutely fine with a module with an explicit method call, but assumed that that wasn't the desire of the original code.

sathish-progress · 2025-10-24T11:02:33Z

  spec.add_dependency "faraday", ">= 1", "< 2"
  spec.add_dependency "faraday-http-cache"
-  spec.add_dependency "activesupport", "~> 7.2", ">= 7.2.2.1"
+  spec.add_dependency "moneta", "~> 1.6"


I understand we are removing activesupport to avoid external dependency issues. But at the same time we are introducing a new dependency that has not received any updates in last 2 years. How can we ensure this receives latest security patches and updates ?

sathish-progress · 2025-10-24T11:07:00Z

@@ -0,0 +1,61 @@
+Test harness notes for the chef-licensing specs


this can go under a testing section in https://github.com/chef/chef-licensing/blob/51652371610c597bdbe919e7ba8c08fd667cf296/components/ruby/README.md

sathish-progress · 2025-10-24T11:16:15Z

Also as discussed with @Nik08 offline, adding Bump Version Minor label to this.

rishichawda · 2025-10-29T05:05:03Z

I've replaced the moneta implementation with pstore. details are in the commit message for dfd8e31 (had to force push my commit because I missed putting the pstore pin info in my commit message). pinning pstore to ~> 0.1.1 because we support ruby 3.1 and that's the pstore version ruby 3.1.x shipped.

cc: @tpowell-progress

…irreconcilable conflicts Signed-off-by: Thomas Powell <thomas.powell@progress.com>

Signed-off-by: Thomas Powell <thomas.powell@progress.com>

Signed-off-by: John McCrae <john.mccrae@progress.com>

moneta seems to be an overkill for our use case. we can do well with pstore instead for storing the key value pairs into file while staying compatible with faraday's http cache interface. this change also adds the pstore gem to gemspec and pins it to ~>0.1.1 because that's the version that was shipped with ruby 3.1.*. Signed-off-by: Rishi Kumar Chawda <rishichawda@users.noreply.github.com>

Signed-off-by: Rishi Kumar Chawda <rishichawda@users.noreply.github.com>

we already have those in buildkite. discussion: https://github.com/chef/chef-licensing/pull/211/files#r2459618792 Signed-off-by: Rishi Kumar Chawda <rishichawda@users.noreply.github.com>

chef-licensing was failing with a `'lookup_middleware': :json is not registered on Faraday::Request` error that was coming from the get connection method in rest client. faraday_middleware gems seems to be the one responsible to provide this json middleware https://github.com/lostisland/faraday_middleware/wiki/Changes-0.8. pinning faraday_middleware to ~>1.0. Signed-off-by: Rishi Kumar Chawda <rishichawda@users.noreply.github.com>

Copilot

Pull Request Overview

This PR removes the activesupport dependency from the chef-licensing gem to resolve dependency conflicts that occur when different projects require incompatible versions of ActiveSupport. The changes implement minimal custom replacements for the specific ActiveSupport features that were being used.

Key changes:

Replace ActiveSupport cache with a custom PStore-based adapter
Implement basic string pluralization using refinements instead of ActiveSupport::Inflector
Update dependencies in gemspec to remove activesupport and add faraday_middleware and pstore

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
chef-licensing.gemspec	Removes activesupport dependency and adds faraday_middleware and pstore dependencies
lib/chef-licensing/pstore_adapter.rb	Implements custom cache adapter using PStore to replace ActiveSupport::Cache
lib/chef-licensing/string_refinements.rb	Provides basic string pluralization to replace ActiveSupport::Inflector
lib/chef-licensing/restful_client/base.rb	Updates to use the new PStore adapter instead of ActiveSupport::Cache
lib/chef-licensing/list_license_keys.rb	Adds usage of the new string refinements module
spec/chef-licensing/activesupport_replacement_spec.rb	Adds comprehensive tests for the new PStore adapter and string refinements
spec/README.md	Documents test setup and best practices for the spec suite
.rubocop.yml	Adds introductory comment

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-03T06:12:58Z

+end
+
+RSpec.describe ChefLicensing::PStoreAdapter do
+  let(:cache_dir) { Dir.mktmpdir }
+  let(:adapter) { described_class.new(cache_dir) }
+
+  after do
+    FileUtils.rm_rf(cache_dir)
+  end
+
+  it "provides the same interface as ActiveSupport::Cache" do
+    adapter.write("test_key", "test_value")
+    expect(adapter.read("test_key")).to eq("test_value")
+    expect(adapter.exist?("test_key")).to be true
+
+    adapter.delete("test_key")
+    expect(adapter.read("test_key")).to be_nil
+  end


This second RSpec.describe block duplicates the PStoreAdapter test setup and should be merged with the first block (lines 5-52) to avoid code duplication. The interface compatibility test can be added as another describe block within the existing structure.

Suggested change

end

RSpec.describe ChefLicensing::PStoreAdapter do

let(:cache_dir) { Dir.mktmpdir }

let(:adapter) { described_class.new(cache_dir) }

after do

FileUtils.rm_rf(cache_dir)

end

it "provides the same interface as ActiveSupport::Cache" do

adapter.write("test_key", "test_value")

expect(adapter.read("test_key")).to eq("test_value")

expect(adapter.exist?("test_key")).to be true

adapter.delete("test_key")

expect(adapter.read("test_key")).to be_nil

end

describe "interface compatibility with ActiveSupport::Cache" do

it "provides the same interface as ActiveSupport::Cache" do

adapter.write("test_key", "test_value")

expect(adapter.read("test_key")).to eq("test_value")

expect(adapter.exist?("test_key")).to be true

adapter.delete("test_key")

expect(adapter.read("test_key")).to be_nil

end

end

Copilot · 2025-11-03T06:12:59Z

+        # Simple pluralization rules
+        case downcase
+        when /s$/, /sh$/, /ch$/, /x$/, /z$/
+          "#{self}es"
+        when /[^aeiou]y$/
+          "#{self[0..-2]}ies"
+        when /f$/
+          "#{self[0..-2]}ves"
+        when /fe$/
+          "#{self[0..-3]}ves"
+        else
+          "#{self}s"


The downcase method is called on the string but the original case is used in the return values. This will cause incorrect capitalization in pluralized strings. For example, 'Day'.pluralize(2) will return 'Days' but should preserve the original case as 'Days'.

Suggested change

# Simple pluralization rules

case downcase

when /s$/, /sh$/, /ch$/, /x$/, /z$/

"#{self}es"

when /[^aeiou]y$/

"#{self[0..-2]}ies"

when /f$/

"#{self[0..-2]}ves"

when /fe$/

"#{self[0..-3]}ves"

else

"#{self}s"

# Determine the pluralized form in lowercase

word = self

plural =

case word.downcase

when /s$/, /sh$/, /ch$/, /x$/, /z$/

"#{word}es"

when /[^aeiou]y$/

"#{word[0..-2]}ies"

when /f$/

"#{word[0..-2]}ves"

when /fe$/

"#{word[0..-3]}ves"

else

"#{word}s"

end

# Preserve original capitalization pattern

if word.match?(/\A[A-Z]+\z/)

plural.upcase

elsif word.match?(/\A[A-Z][a-z]+\z/)

plural.capitalize

elsif word.match?(/\A[a-z]+\z/)

plural.downcase

else

plural

tpowell-progress requested a review from Nik08 October 16, 2025 22:02

jaymzh reviewed Oct 16, 2025

View reviewed changes

Comment thread components/ruby/lib/chef-licensing/cache_adapter.rb Outdated

jaymzh reviewed Oct 16, 2025

View reviewed changes

Comment thread components/ruby/lib/chef-licensing/file_cache.rb Outdated

jaymzh reviewed Oct 16, 2025

View reviewed changes

Comment thread components/ruby/lib/chef-licensing/file_cache.rb Outdated

jaymzh suggested changes Oct 16, 2025

View reviewed changes

Comment thread components/ruby/lib/chef-licensing/string_refinements.rb Outdated

Comment thread components/ruby/lib/chef-licensing/string_refinements.rb Outdated

tpowell-progress force-pushed the tp/remove-activesupport branch from a147247 to b147cb0 Compare October 16, 2025 23:17

johnmccrae approved these changes Oct 17, 2025

View reviewed changes

jaymzh approved these changes Oct 17, 2025

View reviewed changes

sathish-progress suggested changes Oct 24, 2025

View reviewed changes

sathish-progress added the Expeditor: Bump Version Minor label Oct 24, 2025

rishichawda force-pushed the tp/remove-activesupport branch from 2606da1 to dfd8e31 Compare October 29, 2025 04:59

rishichawda requested a review from sathish-progress October 29, 2025 05:05

tpowell-progress and others added 13 commits November 3, 2025 11:30

Remove activesupport due to its volatility and potential to generate …

b284a14

…irreconcilable conflicts Signed-off-by: Thomas Powell <thomas.powell@progress.com>

Use 18.8.46 for rubygems availability

90333d2

Signed-off-by: Thomas Powell <thomas.powell@progress.com>

Disable style for this PR

c3208da

Signed-off-by: Thomas Powell <thomas.powell@progress.com>

Put URL back

0e46287

Signed-off-by: Thomas Powell <thomas.powell@progress.com>

Remove string search

17c84f7

Signed-off-by: Thomas Powell <thomas.powell@progress.com>

Disable activesupport search test

49cc4eb

Signed-off-by: Thomas Powell <thomas.powell@progress.com>

No longer need the activesupport removal check

372cfa0

Signed-off-by: Thomas Powell <thomas.powell@progress.com>

line ending

cc54fb9

Signed-off-by: Thomas Powell <thomas.powell@progress.com>

updating linting to use cookstyle and correct linting errors

cd5337a

Signed-off-by: John McCrae <john.mccrae@progress.com>

fix lint errors

b5fdb6d

Signed-off-by: Rishi Kumar Chawda <rishichawda@users.noreply.github.com>

removed github workflow

b39dec3

we already have those in buildkite. discussion: https://github.com/chef/chef-licensing/pull/211/files#r2459618792 Signed-off-by: Rishi Kumar Chawda <rishichawda@users.noreply.github.com>

rishichawda force-pushed the tp/remove-activesupport branch from 756b506 to a29c0b9 Compare November 3, 2025 06:06

sathish-progress approved these changes Nov 3, 2025

View reviewed changes

sathish-progress requested review from Copilot and removed request for Nik08 November 3, 2025 06:12

Copilot AI reviewed Nov 3, 2025

View reviewed changes

Nik08 approved these changes Nov 3, 2025

View reviewed changes

Nik08 merged commit 8236024 into main Nov 3, 2025
6 checks passed

Nik08 mentioned this pull request Nov 3, 2025

updating ActiveSupport and tests which were failing with licensing ch… #209

Closed

11 tasks

		@@ -0,0 +1,61 @@
		Test harness notes for the chef-licensing specs

-        # Simple pluralization rules
-        case downcase
-        when /s$/, /sh$/, /ch$/, /x$/, /z$/
-          "#{self}es"
-        when /[^aeiou]y$/
-          "#{self[0..-2]}ies"
-        when /f$/
-          "#{self[0..-2]}ves"
-        when /fe$/
-          "#{self[0..-3]}ves"
-        else
-          "#{self}s"
+        # Determine the pluralized form in lowercase
+        word = self
+        plural =
+          case word.downcase
+          when /s$/, /sh$/, /ch$/, /x$/, /z$/
+            "#{word}es"
+          when /[^aeiou]y$/
+            "#{word[0..-2]}ies"
+          when /f$/
+            "#{word[0..-2]}ves"
+          when /fe$/
+            "#{word[0..-3]}ves"
+          else
+            "#{word}s"
+          end
+        # Preserve original capitalization pattern
+        if word.match?(/\A[A-Z]+\z/)
+          plural.upcase
+        elsif word.match?(/\A[A-Z][a-z]+\z/)
+          plural.capitalize
+        elsif word.match?(/\A[a-z]+\z/)
+          plural.downcase
+        else
+          plural

Uh oh!

Conversation

tpowell-progress commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Types of changes

Checklist:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaymzh left a comment

Choose a reason for hiding this comment

Uh oh!

jaymzh commented Oct 17, 2025

Uh oh!

rahulgoel1 commented Oct 22, 2025

Uh oh!

Nik08 commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sathish-progress left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sathish-progress commented Oct 24, 2025

Uh oh!

rishichawda commented Oct 29, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

tpowell-progress commented Oct 16, 2025 •

edited

Loading

Nik08 commented Oct 24, 2025 •

edited

Loading

sathish-progress left a comment •

edited

Loading